-
Notifications
You must be signed in to change notification settings - Fork 13
feat(@netlify/vite-plugin-react-router): add edge support #562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(@netlify/vite-plugin-react-router): add edge support #562
Conversation
✅ Deploy Preview for remix-edge ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for remix-serverless ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
5b9a00c to
b552eb6
Compare
744b059 to
7fde98c
Compare
When the globally installed deno cli isn't a supported version, the build automatically tries to install a local binary. Unfortunately when running multiple builds concurrently on the same machine with default configuration, there's a race condition where a binary tries to get written to the same path where one is currently running, leading to an OS error. Just install the correct version globally. See (internal link) https://netlify.slack.com/archives/C03ETTLQ9BP/p1761826639264259
it turns out... > By default, tag-based purges apply to all of the site’s deploys. To target a specific deploy, > specify one or more of the following [...] from https://docs.netlify.com/build/caching/caching-overview/#use-a-function-with-the-purgecache-helper-to-purge-by-cache-tag Whoops. So concurrent tests were conflicting with one another's expectations.
7fde98c to
78d1afe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File extracted as-is. Only change is the TNetlifyContext generic, so that NFs and EFs can pass in their own slightly different context types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of this file was extracted as is. The only change is the context generic and the runtimeName arg.
| "./function-handler": { | ||
| "types": "./dist/function-handler.d.mts", | ||
| "default": "./dist/function-handler.mjs" | ||
| }, | ||
| "./edge-function-handler": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check out the first lines of FUNCTION_HANDLER and EDGE_FUNCTION_HANDLER to understand what these are for. They're the runtime entry points.
(This should probably always have been an explicit separate export, honestly.)
| > required for the Deno runtime. You may customize your server entry file, but see below for important edge runtime | ||
| > constraints. | ||
| ```tsx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
identical to the Cloudflare one in the RR7 codebase... https://github.com/remix-run/react-router/blob/cb9a090316003988ff367bb2f2d1ef5bd03bd3af/integration/helpers/vite-plugin-cloudflare-template/app/entry.server.tsx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this whole fixture is pretty much identical to the react-router-serverless-site one. I just removed durable, added app/entry.server.tsx, and passed in edge: true and excludedPaths in the vite config.
tests/e2e/fixtures/react-router-serverless-v8-middleware/package.json
Outdated
Show resolved
Hide resolved
When a test retries, it reuses the deployed fixture and since the cache state is preserved from the previous run, the initial state is incorrect. Attaching a unique query string per run isolated each run.
There appears to still be some flakiness that I cannot reproduce when running `.only` on a test, which implies that there's somethhing conflicting somehow across tests running concurrently.
and document a pattern for using `netlifyRouterContext` in edge
fafc121 to
c6d75cb
Compare
By declaring the ESM entries together, we allow code splitting, which keeps the size of our package to a minimum.
This reverts commit 8f54892. It turns out it's the docs that are wrong :(
D'oh. The cache purge calls in the e2e tests were 404ing...
Turns out we had the same problem here as well.
df6e147 to
3beb85c
Compare
Summary
This PR adds opt-in support for deploying React Router 7 apps to Netlify Edge Functions (Deno runtime) via the
edge: booleanplugin option. Previously, the plugin only supported Netlify Functions (Node.js runtime).Toward FRB-1519
Changes
edge?: booleanoption to Vite plugin factoryedgeis enabled:target: 'webworker'and other Deno-compatible build config.netlify/v1/edge-functions/, instead of a function.netlify/v1/functions/excludedPaths?: string[]option to allow users to exclude paths from the React Router handler. Required when usingedge: trueand the project also has its own Netlify Functions with a configuredpath, otherwise only the RR handler runs on all dynamic paths.Testing
Documentation
I added complete documentation for opting in (and back out of) edge deployment.
Implementation
Edge support is more involved than regular serverless functions for three main reasons:
preferStaticoption like NFs.excludedPathwith all published client assets. This skips the EF entirely for these paths.path: '/*', if a user has their own NF withpath: '/foo', our EF will run first and render a response from React Router and the user's NF will never run.excludedPaths?: []option to the plugin.edge: truethe user must include their ownapp/entry.server.tsxthat usesrenderToReadableStreaminstead ofrenderToPipeableStream.